Skip to content
This repository has been archived by the owner on Jan 14, 2024. It is now read-only.

[CVE-2019-1000007] xso: fix parser error handling #268

Merged
merged 1 commit into from Jan 12, 2019
Merged

Conversation

horazont
Copy link
Owner

Needs backport to 0.10!

guard() was incorrectly counting the depth when either of the following was true:

  • the error occured inside the first "start" event on which guard() is used: in that case, guard() would fail to swallow the corresponding "end" event.

  • after an error, further elements appear in the stream before the guard()-ed element is over. in that case, guard() would fail to account for the "start" events caused by those events, and thus let its depth counter go entirely out-of-sync with the XML tree

If this flaw is combined with the use of a supressing xso_error_handler, it is possible to make elements appear higher up in the XML stream tree than they actually are; this implies that it is possible to inject elements in the XML stream.

It requires very specific circumstances for an application to be vulnerable. Example of a vulnerable XSO definition is:

class Baz(aioxmpp.xso.XSO):
    TAG = ("https://xmlns.zombofant.net/aioxmpp/test", "baz")

class Bar(aioxmpp.xso.XSO):
    TAG = ("https://xmlns.zombofant.net/aioxmpp/test", "bar")

    validated = aioxmpp.xso.Attr(
        "foo",
        type_=aioxmpp.xso.JID()
    )

    children = aioxmpp.xso.ChildList([Baz])

@aioxmpp.IQ.as_payload_class
class Foo(aioxmpp.xso.XSO):
    TAG = ("https://xmlns.zombofant.net/aioxmpp/test", "foo")

    child = aioxmpp.xso.Child([Bar])

    def xso_error_handler(self, descriptor, ev_args, exc_info):
        return True

If an attacker sends (formatted for readability):

<iq ... type='result'>
  <foo xmlns='https://xmlns.zombofant.net/aioxmpp/test'>
    <bar foo='&quot;@bar'><baz/><baz/><baz/></bar>
  </foo>
</iq>

to an application which has the above XSO definition loaded, it will see the "end" event of the </iq> on the stream level, breaking the XML stream (because it expects a "start" event instead of an "end" event).

More sophisticated attacks could be used to make an element appear on the stream level instead, which would open the possibility of injecting, for example, <message> stanzas remotely into the stream of a vulnerable aioxmpp client, with arbitrary sender.

guard() was incorrectly counting the depth when either of the
following was true:

- the error occured inside the first "start" event on which guard()
  is used: in that case, guard() would fail to swallow the
  corresponding "end" event.

- after an error, further elements appear in the stream before the
  guard()-ed element is over. in that case, guard() would fail to
  account for the "start" events caused by those events, and thus
  let its depth counter go entirely out-of-sync with the XML tree

If this flaw is combined with the use of a supressing
xso_error_handler, it is possible to make elements appear higher
up in the XML stream tree than they actually are; this implies
that it is possible to inject elements in the XML stream.

It requires very specific circumstances for an application to be
vulnerable. Example of a vulnerable XSO definition is:

class Baz(aioxmpp.xso.XSO):
    TAG = ("https://xmlns.zombofant.net/aioxmpp/test", "baz")

class Bar(aioxmpp.xso.XSO):
    TAG = ("https://xmlns.zombofant.net/aioxmpp/test", "bar")

    validated = aioxmpp.xso.Attr(
        "foo",
        type_=aioxmpp.xso.JID()
    )

    children = aioxmpp.xso.ChildList([Baz])

@aioxmpp.IQ.as_payload_class
class Foo(aioxmpp.xso.XSO):
    TAG = ("https://xmlns.zombofant.net/aioxmpp/test", "foo")

    child = aioxmpp.xso.Child([Bar])

    def xso_error_handler(self, descriptor, ev_args, exc_info):
        return True

If an attacker sends:

    <iq ... type='result'><foo xmlns='https://xmlns.zombofant.net/aioxmpp/test'><bar foo='&quot;@bar'><baz/><baz/><baz/></bar></foo></iq>

to an application, it will see the "end" event of the </iq> *on the
stream level*, breaking the XML stream (because it expects a
"start" event instead of an "end" event).

More sophisticated attacks could be used to make an element appear
on the stream level instead, which would open the possibility of
injecting, for example, <message> stanzas remotely into the stream
of a vulnerable aioxmpp client, with arbitrary sender.
@horazont horazont added the bug Outright bug: e.g. violation of specs, crashes, behaviour contrary to documentation, …. label Jan 10, 2019
@horazont horazont added this to the v0.11 milestone Jan 10, 2019
@horazont horazont self-assigned this Jan 10, 2019
@horazont
Copy link
Owner Author

Backport prepared as f151f92.

@horazont
Copy link
Owner Author

Backport released in v0.10.3.

@horazont horazont merged commit 29ff083 into devel Jan 12, 2019
@horazont
Copy link
Owner Author

This has been assigned CVE-2019-1000007.

@horazont horazont changed the title xso: fix parser error handling [CVE-2019-1000007] xso: fix parser error handling Jan 23, 2019
@horazont horazont deleted the feature/fix-guard branch January 23, 2019 15:33
@horazont horazont restored the feature/fix-guard branch January 26, 2019 15:35
@horazont horazont deleted the feature/fix-guard branch March 10, 2019 11:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Outright bug: e.g. violation of specs, crashes, behaviour contrary to documentation, ….
Projects
None yet
1 participant